Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0][MIG] sale promotion rule #1583

Merged
merged 47 commits into from
Jun 14, 2023
Merged

Conversation

xavier-bouquiaux
Copy link

No description provided.

@rousseldenis
Copy link
Sponsor Contributor

rousseldenis commented May 18, 2021

@hparfr IMHO, either a Roadmap, either a modification before merge should integrate coupon module that has been split by Odoo from 13 => 14 as it could be interesting to base our flow on coupon.coupon model

https:/odoo/odoo/blob/14.0/addons/coupon/models/coupon.py#L10

Not too much logic there, no mandatory fields. What do you think ?

@sebastienbeau @Cedric-Pigeon @qgroulard

@rousseldenis rousseldenis changed the title [WIP][14.0][MIG] sale promotion rule [14.0][MIG] sale promotion rule May 18, 2021
Copy link
Contributor

@hailangvn hailangvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. There are minor improvements not required to merge this.

Comment on lines 54 to 79
<group name="condition" string="Condition" col="3">
<label for="date_from" string="Date début/fin" />
<field name="date_from" nolabel="1" />
<field name="date_to" nolabel="1" />
<field name="minimal_amount" colspan="3" />
<field name="is_minimal_amount_tax_incl" colspan="3" />
<field name="only_newsletter" colspan="3" />
<field
name="restrict_pricelist_ids"
widget="many2many_tags"
colspan="3"
/>
<label for="restrict_partner_ids" colspan="3" />
<field
name="restrict_partner_ids"
nolabel="1"
colspan="3"
/>
</group>
</group>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

                    </group>
                    <group name="condition" string="Condition" col="6">
                        <label for="date_from" string="Date début/fin"/>
                        <field name="date_from" nolabel="1"/>
                        <field name="date_to" nolabel="1"/>
                        <field name="minimal_amount" colspan="3"/>
                        <field name="is_minimal_amount_tax_incl" colspan="3"/>
                        <field name="only_newsletter" colspan="3"/>
                        <field name="restrict_pricelist_ids" widget="many2many_tags" colspan="3"/>
                        <label for="restrict_partner_ids" colspan="6"/>
                        <field name="restrict_partner_ids" nolabel="1" colspan="6"/>
                    </group>

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hailangvn This is reformated through pre-commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rousseldenis. I was new at the time I commented. :)

Comment on lines +1 to +7
<?xml version="1.0" encoding="UTF-8" ?>
<odoo>
<record id="coupon" model="product.product">
<field name="name">Coupon</field>
<field name="type">service</field>
</record>
</odoo>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed with current coupon logic?

@hparfr
Copy link

hparfr commented May 20, 2021

Why not. I was not aware of this coupon module.

@hailangvn
Copy link
Contributor

My comment was about demo coupon product, which IMHO helps new user to understand this module. I was wondering what to do next after having a coupon product while we already added coupon code to promotions page. There is another demo file as well.

<field name="multi_rule_strategy" />
</group>
<group name="condition" string="Condition" col="3">
<label for="date_from" string="Date début/fin" />
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change this to English version ?

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@francesco-ooops
Copy link
Contributor

@xavier-bouquiaux @rousseldenis wouldn't it make more sense to move this module to sale-promotion repo?

let us know if you need us to work on it

@francesco-ooops
Copy link
Contributor

or rather: what are the differences between this module's promotions and odoo standard promotions? should they coexist?

@hparfr
Copy link

hparfr commented May 13, 2022

or rather: what are the differences between this module's promotions and odoo standard promotions? should they coexist?

After few months of implementation with coupons in v14, it's missing key features for our e-commerce case (like apply more than 1 promotion at time). Coupon is not designed to allow behavior changes. So it's a bit hacky to add features on top of it. We spent a lot of effort to make it work.

Sale promotion rule, is an old module date here from 2017 and I think it came from a time when we had to synchronize with Magento / Prestashop and cover main features. It's designed and implemented to allow behavior changes if needed. I think a good refactoring will not hurt.

Like always, it depends of your needs and your budget.

@rousseldenis
Copy link
Sponsor Contributor

@Cedric-Pigeon

@rousseldenis
Copy link
Sponsor Contributor

rousseldenis commented Jun 13, 2022

After few months of implementation with coupons in v14, it's missing key features for our e-commerce case (like apply more than 1 promotion at time). Coupon is not designed to allow behavior changes. So it's a bit hacky to add features on top of it. We spent a lot of effort to make it work.

@hparfr But from 13 version, it is possible to use coupon for its model only and build on top of it as we did here : https:/acsone/acsone-addons/tree/14.0/account_wallet_coupon

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 21, 2023
Copy link
Member

@sebastienbeau sebastienbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (code review), we should merge it even odoo have an alternative implementation (coupon do not solve all case). In a long terme we should see if we can use coupon or improve this module.
For now some customer need such feature so let's merge it

<field name="multi_rule_strategy" />
</group>
<group name="condition" string="Condition" col="3">
<label for="date_from" string="Date début/fin" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bguillot and others added 4 commits June 14, 2023 09:53
The module now supports multiple promotion rules.
When a coupon is used, it always take precedence on automatic promotion
rule.
The computation of promotions has been refactored to update lines from
the sale.order object. Sine lines are updated from the SO, the recompute
of SO fields that depends of lines is only tiggered at the end of the
update on the lines. (In place of each time a line is updated).
You can ask to recompute the promotion of a sale.order. If a coupon is
already specified it's conserved but all the automatic rules are applied
again.
rousseldenis and others added 18 commits June 14, 2023 09:53
If the coupon rule apply a discount in fixed price, the rule should
not be applied on each sale order line
Sequence order was never applied du of missing _order

[UPD] Update sale_promotion_rule.pot
Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-10.0/sale-workflow-10.0-sale_promotion_rule
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-10-0/sale-workflow-10-0-sale_promotion_rule/

[UPD] Update sale_promotion_rule.pot

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-10.0/sale-workflow-10.0-sale_promotion_rule
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-10-0/sale-workflow-10-0-sale_promotion_rule/

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-10.0/sale-workflow-10.0-sale_promotion_rule
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-10-0/sale-workflow-10-0-sale_promotion_rule/
If no rule is percentage (i.e. edit discount field) we should not clear the discount when removing promotions.
Fix tests using demo data modified by other modules
@xavier-bouquiaux
Copy link
Author

LGTM (code review), we should merge it even odoo have an alternative implementation (coupon do not solve all case). In a long terme we should see if we can use coupon or improve this module. For now some customer need such feature so let's merge it

Changed to english version and rebased

regards

xbo

@sebastienbeau
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-1583-by-sebastienbeau-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 259d4fd into OCA:14.0 Jun 14, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at c9dc6ab. Thanks a lot for contributing to OCA. ❤️

@acsonefho acsonefho deleted the 14-sale_promotion_rule branch June 14, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved merged 🎉 migration needs review stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.